Mark merge conflict comments as resolved once the conflict is fixed - #20
Open
diedexx wants to merge 5 commits into
Open
Mark merge conflict comments as resolved once the conflict is fixed#20diedexx wants to merge 5 commits into
diedexx wants to merge 5 commits into
Conversation
The "Create label if it doesn't exist" step interpolated the caller-controlled `dirtyLabel` input directly into the `actions/github-script` body. Because this workflow is reusable and is typically called from a `pull_request_target` trigger, the step runs with a write-scoped token, so a label name crafted to break out of the surrounding string literal could execute arbitrary code in the context of the calling repository. Pass the input through the environment and read it via `process.env` instead, so the value is always treated as inert data rather than as part of the script. Reported by zizmor as template-injection (high severity). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge conflict check already removes the dirty label when a conflict is resolved, but the comment explaining the conflict stayed visible, leaving stale noise on the PR. Minimize it as RESOLVED instead, which collapses it the same way the "Hide comment" UI does. The check step's existing `prDirtyStatuses` output is reused to determine which PRs are no longer conflicting, so no extra merge state queries are needed. Only comments which were authored by this workflow, are not already minimized, and whose body matches `commentOnDirty` exactly are minimized, so human comments and unrelated bot comments are never hidden. Pagination stops when the API claims a further page without advancing the cursor, which would otherwise loop forever, and the job now has an explicit timeout as a second line of defense. Permissions are declared explicitly and default to none at the workflow level, so the job only gets the scopes it actually needs. The behavior is opt-out via the new `minimizeCommentOnClean` input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cope Code review raised two assumptions worth recording. Minimizing relies on an exact match against `commentOnDirty`, so editing that input leaves comments which were posted with the previous wording visible instead of collapsing them. Mention this in the README so the behavior is not surprising. Also note that `viewerDidAuthor` filters on the identity behind the token, which is the default GITHUB_TOKEN here, so a future change passing a PAT or app token to that step would widen which comments get minimized. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the reusable merge-conflict-check workflow to reduce stale PR noise by auto-collapsing prior “merge conflict” bot comments after conflicts are resolved, and hardens the workflow against script injection in a label-creation step.
Changes:
- Adds an opt-out input (
minimizeCommentOnClean) to minimize the priorcommentOnDirtycomment as RESOLVED when a PR becomes conflict-free. - Fixes a script-injection vector by passing caller-controlled inputs via
envand reading them throughprocess.envinactions/github-script. - Narrows token permissions (workflow-level
permissions: {}, job-level least-privilege) and adds a job timeout safety net.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| README.md | Documents the new minimizeCommentOnClean input and its behavior. |
| .github/workflows/reusable-merge-conflict-check.yml | Implements comment minimization logic, adds explicit permissions + timeout, and fixes input interpolation injection risk. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The conflict comment is usually one of the most recent comments on a PR, but the scan walked the comment history from the oldest comment forward, so PRs with a long history needed several requests to reach it. Walk the comments backwards instead, which normally finds the comment in the first page. The endless loop guard now tracks `startCursor`/`hasPreviousPage` accordingly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
diedexx
marked this pull request as ready for review
July 28, 2026 12:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The merge conflict check already removes the
merge conflictlabel once a PR stops conflicting, but the comment explaining the conflict stayed visible. That left stale, misleading noise on PRs that were no longer conflicting.This collapses that comment as
RESOLVEDonce the conflict is gone, which is the same result as using the "Hide comment" UI manually.While working on this,
zizmorflagged a pre-existing high severity script injection vulnerability in the "Create label" step, which is fixed here as a separate commit.Changes
Security fix (39ad453)
${{ inputs.dirtyLabel }}directly into anactions/github-scriptbody. Since this workflow is reusable and is typically called frompull_request_targetwith a write-scoped token, a crafted label name could break out of the string literal and execute arbitrary code in the calling repository. The input is now passed through the environment and read viaprocess.env, so it is always treated as inert data.Auto-resolve comments (a244059)
commentOnDirtycomment as resolved via the GraphQLminimizeCommentmutation once a PR is no longer conflicting.commentOnDirtyexactly are minimized.prDirtyStatusesoutput to decide which PRs are clean, so no extra merge state queries are needed.timeout-minutes: 20as a last line of defense against a runaway job.permissions: {}at workflow level, with the job opting in to onlyissues: writeandpull-requests: write. This narrows the token compared to the previous implicit defaults.